Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FSSDK-8983] Specify package entry point using exports field #836

Merged
merged 4 commits into from
Jan 18, 2024
Merged

[FSSDK-8983] Specify package entry point using exports field #836

merged 4 commits into from
Jan 18, 2024

Conversation

raju-opti
Copy link
Contributor

@raju-opti raju-opti commented Jul 21, 2023 •
edited
Loading

Summary

  • specified package entry points using the modern exports field in package.json
  • added an export for lite bundle. lite bundle can be now imported using import optimizely from '@optimizely/optimizely-sdk/lite

Test plan

  • Tested that the new export entry points are working as expected. Added specific logs to the built files in the dist folder for each specific variant. Then tested that the specific variant is being loaded by installing that locally built sdk in a test project and inspecting the console log. Tested for both browser and node.

Issues

  • FSSDK-8983

@raju-opti raju-opti requested a review from a team as a code owner July 21, 2023 13:34
@coveralls
Copy link

coveralls commented Jul 21, 2023 •
edited
Loading

Coverage Status

coverage: 90.411%. remained the same
when pulling a93990b on raju/package-export-path
into f61dc52 on master.

mikechu-optimizely
mikechu-optimizely requested changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@mikechu-optimizely mikechu-optimizely left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of items/thoughts.

packages/optimizely-sdk/package.json Outdated Show resolved Hide resolved
packages/optimizely-sdk/package.json Outdated Show resolved Hide resolved
packages/optimizely-sdk/package.json Outdated Show resolved Hide resolved
@mikechu-optimizely
Copy link
Contributor

@zashraf1985 Can you do a light review when you get a chance?

zashraf1985
zashraf1985 approved these changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@zashraf1985 zashraf1985 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks great! But i have no idea about this new feature so i would recommend thorough testing using all possible known variants of user applications / supported JS versions before moving forward.

mikechu-optimizely
mikechu-optimizely approved these changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@mikechu-optimizely mikechu-optimizely left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Zeeshan says ok 😁

Please ensure all checks are passing.

@mikechu-optimizely
Copy link
Contributor

Labeled this as hold pending a ODP release.... 🤔 Right @raju-opti ?

@mikechu-optimizely mikechu-optimizely mentioned this pull request Aug 24, 2023
Closed
@mikechu-optimizely
Copy link
Contributor

@raju-opti is this a good time to bring this PR into the code before AAT release?

@raju-opti
Copy link
Contributor Author

@raju-opti is this a good time to bring this PR into the code before AAT release?

Yeah, I think it is!

@mikechu-optimizely
Copy link
Contributor

@raju-opti is this a good time to bring this PR into the code before AAT release?

Yeah, I think it is!

I did a quick review again of the file change and still LGTM. When you get a chance, please update and merge.

@raju-opti raju-opti force-pushed the raju/package-export-path branch from 5397439 to a93990b Compare January 18, 2024 19:42
@raju-opti raju-opti merged commit af1ebd2 into master Jan 18, 2024
18 checks passed
@raju-opti raju-opti deleted the raju/package-export-path branch January 18, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikechu-optimizely mikechu-optimizely mikechu-optimizely approved these changes

@zashraf1985 zashraf1985 zashraf1985 approved these changes

Assignees

@mikechu-optimizely mikechu-optimizely

@raju-opti raju-opti

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@raju-opti @coveralls @mikechu-optimizely @zashraf1985